Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ad hoc: Band-aid fix for clear peer list. Should fix #7331 #8975

Merged
merged 1 commit into from
Nov 28, 2016

Conversation

tywald
Copy link
Contributor

@tywald tywald commented Sep 13, 2016

This is a temporary solution.

@hrydgard
Copy link
Owner

I haven't done much with the peer code, but there must be a reason someone commented those lines out. Is this well tested with multiple games? If so I'm fine with merging.

@LunaMoo
Copy link
Collaborator

LunaMoo commented Sep 14, 2016

This was commented out with 1c7152d
which was part of #7264

Hate the fact that soo much things in this code are commented out without actual comment on reason... Just a guess, but maybe some games are disconnecting too early hence it was disabled to make it playable as improved compatibility was primary reason behind people wanting that pr to be merged.

Feels risky to merge right before release.

@tywald
Copy link
Contributor Author

tywald commented Sep 14, 2016

Unfortunately I have only tested with Monster Hunter games, they work at least. During a multiplayer session it should be working normally as before because they are only called when you disconnect, sceNetAdhocctlDisconnect(). So it shouldn't remove any peer prior to that. And if you disconnect you would need to reconnect anyway so the peers would be added again.

I checked if deleteFriendByIP() could cause an exception if friends == NULL but luckily the for-loop only runs when it's != NULL.

Code wise it doesn't seem to be a problem but who knows what the games can do.

@unknownbrackets unknownbrackets added this to the Future milestone Sep 15, 2016
@adenovan
Copy link
Contributor

adenovan commented Sep 23, 2016

Game that not affected by this pull request all game below working normally

  • Monster Hunter Freedom Unite
  • Monster Hunter Portable 3RD
  • God Eater Burst
  • God Eater 2
  • Yu-Gi-Oh! 5D's Tag Force 6
  • Dissidia 012 Duodecim Final Fantasy
  • More test coming

@hrydgard
Copy link
Owner

@adenovan How do you feel about this pull request? I don't do much network gaming but if you've tested it with a lot of games, I'm fine with merging this. Code-wise, it seems correct.

@adenovan
Copy link
Contributor

@hrydgard i feel this PR is safe to merge because almost all of my games work without problem but it still a few list of game if compared with adhoc compability list on forums. i will ask facebook openvpn comunity to do more test on game that i didn't have. so i can list what game call this cleared peer list function and break the networking that works before. this pull request is really needed for Monster Hunter game because #7331 is really an annoying bug specially for playing online with random people. the list on my previous comment will be updated for the info what game work and what game broken by this PR.

@tywald
Copy link
Contributor Author

tywald commented Nov 28, 2016

@adenovan Just poking to check if there has been more tests recently.

What kind of games that many people play which are not on the list above are left to be tested? I assume Tekken 6 should be checked before concluding. It seems good so far, no broken games yet.

@hrydgard
Copy link
Owner

Let's go for it and see if there are complaints instead, at this point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants